방금 작성한 내 코드를 읽는 것도 쉽지 않은데, 다른 사람의 코드를 읽는 것은 당연히 어렵다. 게다가 다른 사람이 작성한 코드를 리뷰하고 의견을 줘야 한다면 어떨까? 코드 리뷰를 한다고 했을 때 무엇을 하면 좋은지, 어떻게 하면 잘 할 수 있는지 이야기 해보려고 한다.
일종의 코드 리뷰 가이드 문서라고 할 수 있을 것 같은데, 이론과 스킬(?) 그리고 실전을 나눠서 구성해봤다.
I. 이론
리뷰의 방향은 어디로 가야 하는지, 대상은 무엇인지 먼저 이야기 해보려고 한다.
무엇을 리뷰하는가?
리뷰 대상인 코드는 두 가지 관점으로 바라볼 수 있다.
- 기능을 동작하도록 구현했는가
- 기능을 동작하면서 잘 구현했는가
얼핏 보면 같은 말 같지만 조금 다르다. 기능이 동작하도록 구현하는 것은 사실 그렇게 어렵지 않다. 하지만 기능을 좋은 코드로 좋은 설계로 잘 구현하는 것은 다른 수준의 이야기이다. 좋은 코드, 좋은 설계가 필요한 이유는 소프트웨어는 끊임없이 변경되기 때문이고 생애 주기가 긴 소프트웨어 일수록 변경에 유연한 설계로 개발되는 것이 중요하다.
'비'기능적 요구사항
소프트웨어를 구성하는 코드는 '좋은 코드'로 구성되어야 하고 이를 요구사항으로 정리한 것을 비기능적 요구사항이라고 하자. 예를 들면 다음과 같은 것들이다.
- 유지보수성
- 테스트 용이성
- 재사용 가능성
- 가독성
도메인과 비즈니스
그리고 기능적 요구사항은 흔히 말하는 도메인 로직, 비즈니스 로직을 의미한다. 조직이 커질수록 리뷰 대상이 되는 모든 제품의 비즈니스 상황을 이해하는 것은 거의 불가능에 가깝다. 그렇기 때문에 도메인과 비즈니스 로직에 대한 리뷰는 쉽지 않다.
그렇다고 해서 비기능적 요구 사항에 대한 리뷰도 함께 불가능해지는 것은 아니다. 이 두 가지 관점을 함께 리뷰하려고 하다 보니 코드 리뷰가 어려워지는 것은 아닐까? 그렇기 때문에 우선 비기능적 요구사항에 집중하면 어떨까?
도메인에 대한 이해가 없다면 비기능적 요구사항을 리뷰하자.
어떻게 리뷰하는가?
앞서 소개한 비기능적 요구사항을 어떻게 리뷰할 수 있을지 구체적인 이야기로 넘어가자. 비기능적 요구사항은 거시적 관점과 미시적 관점, 이 두 가지 관점으로 살펴볼 수 있다.
계층 관점 리뷰 (거시적 관점)
기본적으로 소프트웨어 애플리케이션은 한 가지 기능을 여러 계층으로 나누어 구현한다. 이 계층은 어떻게 구분짓느냐에 따라 더 세분화 할 수 있고 계층의 개수가 달라지겠지만 크게 다음과 같이 나눠볼 수 있다.
- 비즈니스 로직 레이어
- 비즈니스 로직 구현을 위한 모듈 레이어 (추상화 레이어들)
- 모듈을 구성하는 라이브러리, 프로그래밍 언어 레이어
팀에서 합의한 또는 프로젝트, 제품에서 구성하고 있는 계층이 있을텐데, 그 계층에 맞게 코드가 배치되었는지 보는 것이 계층 관점의 리뷰이다. 코드의 배치가 곧 설계라고 할 수 있다. 코드가 알맞은 곳에 잘 배치되어 있어야 다음 수행할 미시적인 관점의 리뷰가 의미있기 때문에 이 관점의 리뷰가 더 중요하다. 다음과 같은 것들을 살펴보고 코멘트 할 수 있다.
- 동일한 레벨의 코드가 사용되었는지 (예: 비즈니스 계층에서는 추상화 레이어의 코드를 사용)
- 인접 계층을 참조하는 것이 아닌, 계층을 건너서 참조하고 있진 않은지
- 비즈니스 로직을 구성하는 코드들이 응집도있게 구성되어 있는지
나쁜 코드 냄새 맡기 (미시적 관점)
계층 내부로 들어가서 조금 더 자세히 들여다보면 코드 레벨의 리뷰를 진행하기 된다. 아래와 같은 항목들을 중심으로 살펴보자.
1. 모듈의 구현이 노출되지 않았는가?
구현의 노출은 변경에 취약하게 만든다. 모듈은 기본적으로 필요한 것만 노출하는 것이 좋다. 불필요한 것은 숨기고 모듈이 제공하는 기능에 대해서만 인터페이스를 두고 노출하는 것이다.
모듈 내부에서 관리하고 있는 데이터가 있다면 그 데이터의 생김새, 자료구조가 노출되었는지 확인해보자. 자료구조는 구현의 일부라고 할 수 있다. 자료구조를 조작하는 오퍼레이터(operator) 또한 자료구조에 의존하고 있기 때문에 마찬가지의 이유로 노출되면 변경에 취약해진다.
- 외부에서 사용하지 않는 것을 불필요하게 노출하였는가?
- 노출을 의도하지 않았지만 노출되었는가?
- 노출을 했다면 올바른 인터페이스로, 관계를 잘 드러낸 채로 노출하였는가?
2. 구현을 봐야 어떤 기능인지 알 수 있는가?
합성, 조합을 우선적으로 고려하자.
모듈의 이름과 인터페이스의 조합으로 어떤 기능을 하는지 예측할 수 있는데, 너무 많은 기능을 할 경우 구현이 복잡해질 뿐만 아니라 구현을 봐야만 어떤 기능인지 알 수 있다. 이름을 짓기 어려운 모듈은 대부분 많은 역할을 하고 있다.
높아진 복잡도를 개선하기 위해 하나의 구현체로 추출하고 그 안에서 또다른 구현체로 추출하게 되면서 '드릴링(Drilling)'이 발생하게 되는데, 하나의 모듈을 이해하기 위해 여러 맥락을 들어가면서 파악해야 하기 때문에 복잡도는 더 높아지게 된다.
각각의 작은 인터페이스로 모듈을 구성하고 이들을 조합하여 구성하는 방식으로 개선해야 한다. 조합을 우선적으로 고려하다보면 코드가 이해하기 쉬워지고 만들어진 모듈들은 재사용성이 높아지게 된다.
- 내부 구현을 계속해서 추적해야 기능을 파악할 수 있는가?
- 인터페이스는 기능을 잘 설명할 수 있는가?
- 함수 시그니처를 통해 그 역할을 예상할 수 있는가?
- 모듈이 한 가지 역할을 하고 있는가?
암시적인 것보다 명시적인 게 좋다.
인터페이스에 전달하는 인자도 많아지게 되면서 기본 인자 (Default Parameter)라는 문법을 고려하게 된다. 잘 사용하면 좋지만 이것이 암시적으로 기능을 담당하게 될 경우, 예상하기 어려워진다. 오버라이드(override) 개념으로 사용할 경우에 사용하자.
- 되도록 인자를 전달하여 명시적으로 의도를 드러내자.
3. 자연스럽지 않은 코드가 있는가?
흘러가듯이 자연스럽게 작성되지 않은 코드가 가끔 보인다. 이럴 땐 나쁜 코드의 냄새가 날 수 있으므로 유심히 들여다보자.
모듈이 사용되는 곳을 전제하고 있진 않은가?
추출을 하다보면 모듈이 사용되는 곳의 도메인을 품고 추출되는 경우가 존재한다. 이럴 경우 모듈은 알 필요가 없는 도메인을 알게 되고 이러다 보면 이해하기 어렵고 자연스럽지 않은 인터페이스가 만들어진다.
예를 들면, 대출과 관련된 컴포넌트를 개발하는데, 대출 금액을 읽기 쉽게 한국어로 만들어주는 부분이 있다고 가정하자. formatLoanToKorean이라는 함수를 만들어 볼 수 있을 것이다. 이 때 조금 더 고민해보면 이 함수는 loan이라는 대출이라는 맥락을 알고 있을 필요가 없다. 단순히 숫자를 한국어로 포맷팅해주는 함수이기 때문이다.
도메인을 알 필요가 없는 모듈은 일반적인 인터페이스로 개선해서 기능을 잘 드러내도록 하자.
타입은 추론되는게 좋다.
TypeScript에서 as keyword를 사용하여 타입을 변환한다던가, Generic에 넘겨줘야만 하는 경우가 있다던가 할 때 추론되는 코드는 아닌지 살펴보자
eslint는 최대한 지키는 것이 좋다.
eslint의 규칙을 ignore하고 작성한 코드가 있다면 무시하지 않고 작성할 수는 없을지 살펴보자. (대표적인 예: useEffect dependency array ignore rule)
hacky한 코드가 포함된다면 주석과 함께 작성되는 것이 좋다.
그 외에도 좋은 코드, 좋은 컴포넌트의 관점에서 볼 수 있는 부분이 많다. 자신만의 관점을 만들어나가자.
II. 스킬
앞서 살펴본 이론들을 적용하는 것은 또 다른 이야기이다. 실제 Pull Request(or Merge Request)가 올라왔을 때, 어떻게 접근하면 효율적으로 리뷰할 수 있을지 이야기해보려고 한다.
1. Pull Request Body (본문) 읽기
당연하지만 꽤 중요하다. 이 변경 사항이 어떤 기능을 수정했는지 구현했는지 이해하는데 집중한다. 리뷰 요청자가 작성한 내용에 따라 이해할 수 있는 정도가 천차만별이지만, 가장 먼저 확인해봐야 하는 내용이다. 더 나아가 해결하고자 했던 문제가 무엇인지 한번 더 고민해보고 이 문제를 가장 값 싸게, 적은 코드로 해결했는지도 살펴볼 수도 있다.
2. 변경 사항의 전체적인 구조를 살펴본다.
앞서 살펴본 PR Body가 어떻게 구현되었는지 살펴보자. GitHub 기준, File Changed 화면으로 넘어가면 pr body를 볼 수 없는데 이게 참 불편하다. 그래서 File Changed는 새 창으로 열어 한쪽 화면은 PR Body를 띄우고 한쪽 화면은 File Changed를 띄워 기능이 어떻게 작성됐는지 매칭시키면서 확인한다.
- 디렉토리 구조와 파일명으로부터 이 PR에서 구현한 기능이 예상되는지 살펴보기
- 각 모듈들이 한 가지 역할을 하게 끔 구성됐는지 살펴보기
3. 사용하는 곳부터 살펴보기 (인터페이스)
웹 애플리케이션의 경우, 페이지 단위로 루트 컴포넌트를 구성하곤 하는데 변경된 파일 목록 중, 루트 컴포넌트가 있다면 해당 컴포넌트 먼저 확인하는 것이 좋다. 위에서 아래 방향으로 코드를 읽어내려가면서 인터페이스를 먼저 살펴보기 위해서다. 컴포넌트라면 Props를 살펴볼 수 있고 함수라면 함수의 시그니처를 먼저 살펴보고 제공하는 기능을 파악할 수 있어야 한다.
4. 모듈의 구현 살펴보기
사실상 버그가 가장 많이 나오는 부분이 이 부분이지만, 이 부분까지 세세하게 리뷰하기가 어렵다. 물론 branch를 checkout 받아 로컬 개발 환경에서 직접 테스트해보며 리뷰를 할 수도 있다. 하지만 비용이 많이 들어가기 때문에 모든 변경 사항에 대해 이렇게 리뷰하기는 현실적으로 힘들다.
그렇기 때문에 문제를 해결하기 위한 라이브러리 API 추천, 좀 더 나은 Syntax 사용 제안 등의 리뷰가 주로 이뤄진다.
III. 실전
당연하게도 이 글을 읽고 나서 바로 리뷰가 수월해지진 않는다.
잘 요청하기
리뷰를 잘하기 위해선 먼저 리뷰를 잘 요청하는게 먼저이다 그렇기 때문에 잘 요청하는 연습으로 시작하면 좋다. 앞서 이야기 한 변경 사항에 대한 설명을 자세하게 적는 것부터가 시작이다. 가장 저비용으로 시작해볼 수 있는 것은 구현한 화면의 스크린샷을 포함하는 것이다. 가장 좋은 것은 해당 커밋에 대한 변경 사항을 preview 할 수 있어서 직접 경험해볼 수 있는 환경을 구성해두는 것이다.
1 thing 1 PR
한 리뷰에는 되도록 한 가지 기능만 올리면 좋다. 어떤 기능을 어떻게 구현했는지 1:1로 연결지어서 리뷰하기 수월해진다. 작은 단위로 리뷰하기 위해 커밋 단위로 리뷰를 진행하기도 한다.
사용하는 부분과 함께
리뷰 대상이 되는 코드 양이 적을 수록 좋긴 하지만, 구현부와 사용부는 함께 다루면 좋다. 인터페이스를 먼저 살펴보고 나서 구현을 살펴봄으로써 인터페이스는 직관적인지, 역할이 제대로 나뉘어졌는지를 파악할 수 있다. 구현체만 올리게 되면 어떻게 사용되는지 알기 어렵고 구현을 먼저 봐야하기 때문에 데이터의 흐름과 인터페이스에 대한 리뷰가 어렵다.
테스트 코드로 사용하는 부분의 코드를 작성할 수 있다. 구현체가 어떻게 사용되는지, 어떤 기능을 담당하고 있는지 별도의 설명없이 테스트 코드만 봐도 이해할 수 있고 여러 상황에 대한 케이스도 살펴볼 수 있어서 리뷰하기 좋다.
연습
앞서 이야기한 것들을 적용해보는 연습을 해봐야 리뷰를 그나마 잘 할 수 있게 된다. 의도적으로 연습하고 연습을 위한 시간 투자를 하자. 요청을 한 시점에 리뷰하면 좋다. 작업의 흐름 상 리뷰를 요청한 시점, Pull Request를 올린 시점이 맥락이 전환된 시점이다. 다른 작업에 집중하러 가기 전에, 요청이 올라온 Pull Requeset를 리뷰하자.
질문
개선을 제안하는 코멘트만 코드 리뷰일가. 코드 리뷰의 목적이 관점을 공유하는 과정임을 인지한다면 질문 또한 리뷰가 될 수 있다. 의도가 이해되지 않거나 궁금한 부분이 있다면 질문을 통해 관점을 공유할 수 있다. 의문이 드는 코드라면 개선할 지점이 있다는 신호일 수도 있기 때문에 리뷰가 어렵다면 질문으로 시작하는 것도 좋다고 생각한다.
학습
사실 “어떻게”에 대한 부분은 좋은 코드에 대한 이해가 많으면 많을 수록 다양한 관점이 생긴다. 이런 관점은 좋은 코드를 많이 보고 읽고 이를 기반으로 고민을 해봐야 얻을 수 있다. 구현과 기능의 차이에 대해 고민해보는 것이 도움이 됐다. 좋은 책이 참 많지만 우선 '오브젝트', '함수형 사고' 두 책을 추천한다. ('클린코드', '리팩토링' 등 기본서들도 당연히 많이 도움된다.)
책을 통해 공부하는 것도 물론 좋지만 결국 코드를 많이 읽어야 하는 것이 중요하다. 읽은 코드는 GitHub 오픈소스에 널려있다. 이 코드가 왜 이렇게 작성되었는지, 어떤 의사결정을 통해 나온 산출물인지도 알 수 있다.
리뷰어 그룹
앞서 비기능적 요구 사항에 집중하여 리뷰하고 기능적 요구 사항은 개인의 역량에 맡기자라는 이야기를 했다. 오히려 도메인에 대해 알지 못하는 것이 비기능적 요구사항에 집중할 수 있는 환경을 만들어주기도 한다. 하지만 기능적 요구사항에 대해서도 리뷰가 필요하다면 그룹을 만들어서 시도해볼 수 있다. 같은 도메인을 개발하는 엔지니어들을 그룹으로 만들어서 이 그룹 내에선 도메인과 관련된 리뷰를 중점적으로 해볼 수 있지 않을까?
마무리
이전엔 코드 리뷰의 목적에 대해서 이야기했다면 이번 글에서는 그래서 코드 리뷰를 어떻게 할 것인지에 대해 다뤄봤다. 코드 리뷰는 함께 성장하는 개발 문화를 구성하는데 있어서 큰 부분을 차지한다고 생각한다. 좋은 활동이 많은 팀에 잘 정착되길 바란다.